Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AMORO-1906]Refactor code of "Table" and "Catalog" to support Paimon format integration #1960

Merged
merged 39 commits into from
Oct 10, 2023

Conversation

shidayang
Copy link
Contributor

Why are the changes needed?

Close #1906.

Brief change log

  1. Replace all ArcticTable with AmoroTable.
  2. Refactor 'orphan file clean' and 'snapshot expire'

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

…copy

# Conflicts:
#	ams/server/src/main/java/com/netease/arctic/server/table/executor/OrphanFilesCleaningExecutor.java
#	ams/server/src/main/java/com/netease/arctic/server/table/executor/SnapshotsExpiringExecutor.java
#	ams/server/src/test/java/com/netease/arctic/server/table/executor/TestSnapshotExpire.java
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Attention: 458 lines in your changes are missing coverage. Please review.

Files Coverage Δ
...etease/arctic/server/DefaultOptimizingService.java 22.15% <ø> (ø)
.../netease/arctic/server/catalog/CatalogBuilder.java 43.75% <100.00%> (+10.41%) ⬆️
...ease/arctic/server/catalog/IcebergCatalogImpl.java 73.33% <100.00%> (+27.17%) ⬆️
...tic/server/catalog/InternalIcebergCatalogImpl.java 89.28% <100.00%> (+0.39%) ⬆️
...arctic/server/catalog/MixedIcebergCatalogImpl.java 73.68% <100.00%> (+1.46%) ⬆️
...m/netease/arctic/server/catalog/ServerCatalog.java 100.00% <ø> (ø)
...ease/arctic/server/optimizing/OptimizingQueue.java 73.81% <100.00%> (+0.14%) ⬆️
...tease/arctic/server/table/DefaultTableService.java 85.82% <100.00%> (-0.77%) ⬇️
.../com/netease/arctic/server/table/TableManager.java 0.00% <ø> (ø)
.../com/netease/arctic/server/table/TableRuntime.java 74.50% <100.00%> (+0.41%) ⬆️
... and 34 more

... and 33 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@shidayang shidayang changed the title Refactor code of "Table" and "Catalog" to support Paimon format integration [AMORO-1906]Refactor code of "Table" and "Catalog" to support Paimon format integration Sep 14, 2023
@shidayang shidayang requested a review from majin1102 September 15, 2023 03:24
…copy

# Conflicts:
#	ams/server/src/main/java/com/netease/arctic/server/table/executor/OrphanFilesCleaningExecutor.java
…copy

# Conflicts:
#	ams/server/src/main/java/com/netease/arctic/server/catalog/MixedIcebergCatalogImpl.java
#	ams/server/src/main/java/com/netease/arctic/server/dashboard/ServerTableDescriptor.java
#	ams/server/src/main/java/com/netease/arctic/server/table/executor/OrphanFilesCleaningExecutor.java
#	ams/server/src/main/java/com/netease/arctic/server/table/executor/SnapshotsExpiringExecutor.java
#	ams/server/src/main/java/com/netease/arctic/server/table/executor/TableRuntimeRefreshExecutor.java
#	core/src/main/java/com/netease/arctic/formats/mixed/MixedHiveTable.java
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some commnets, PTAL.

@github-actions github-actions bot added the module:mixed-hive Hive moduel for Mixed Format label Oct 9, 2023
@@ -80,7 +47,10 @@ protected long getNextExecutingTime(TableRuntime tableRuntime) {

@Override
protected boolean enabled(TableRuntime tableRuntime) {
return tableRuntime.getTableConfiguration().isExpireSnapshotEnabled();
return tableRuntime.getFormat() == TableFormat.ICEBERG &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does all the Paimon format tables not require services provided by TableExecutor? If so, can we move the filter logic at a higher level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executor is a fundamental mechanism, and it's difficult to guarantee that Paimon won't use it in the future

shidayang and others added 5 commits October 9, 2023 19:27
…ixedAndIcebergTableDescriptor.java

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
…aimonTableDescriptor.java

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
@github-actions github-actions bot added the module:ams-server Ams server module label Oct 9, 2023
Copy link
Contributor

@zhoujinsong zhoujinsong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Thanks a lot for your work!

@zhoujinsong zhoujinsong merged commit 4ae100a into apache:master Oct 10, 2023
5 of 6 checks passed
ShawHee pushed a commit to ShawHee/arctic that referenced this pull request Dec 29, 2023
…format integration (apache#1960)

* Integration paimon

* Integration paimon

* Integration paimon

* Integration paimon

* tmp

* Support registering for the Paimon Catalog

* Support registering for the Paimon Catalog

* Support registering for the Paimon Catalog

* Support registering for the Paimon Catalog

* Fix ut

* Polish code

* Fix compile error

* Merge master

* polish code

* Merge master

* Fix UT

* Polish code

* Polish code

* polish code

* polish code

* Update ams/server/src/main/java/com/netease/arctic/server/dashboard/MixedAndIcebergTableDescriptor.java

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>

* Update ams/server/src/main/java/com/netease/arctic/server/dashboard/PaimonTableDescriptor.java

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>

* tmp

* polish code

* polish code

---------

Co-authored-by: ZhouJinsong <zhoujinsong0505@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:ams-dashboard Ams dashboard module module:ams-server Ams server module module:core Core module module:mixed-hive Hive moduel for Mixed Format type:build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Subtask]: Refactor code of "Table" and "Catalog" to support Paimon format integration.
4 participants